From: Gergő Tisza <tgr.huwiki@gmail.com>
Date: Thu, 20 Jun 2019 15:17:37 +0000 (+0200)
Subject: ResponseFactory: support 302 redirects
X-Git-Tag: 1.34.0-rc.0~1276^2
X-Git-Url: http://git.cyclocoop.org/%22.%24info%5B?a=commitdiff_plain;h=19ad413d5dbbe15cfd6c5860f694e07e9abf8031;p=lhc%2Fweb%2Fwiklou.git

ResponseFactory: support 302 redirects

Needed to match existing Parsoid behavior.

Also fixes redirect factory methods mistaking claiming to support
relative URLs. Most clients accept a relative URL in the Location
header, but the spec requires an absolute one, so better say that.

Change-Id: I03f5e776f7629eff6440698655277d8cd01e4a15
---

diff --git a/includes/Rest/ResponseFactory.php b/includes/Rest/ResponseFactory.php
index 7ccb612748..d18cdb5d6b 100644
--- a/includes/Rest/ResponseFactory.php
+++ b/includes/Rest/ResponseFactory.php
@@ -78,7 +78,7 @@ class ResponseFactory {
 	 * the new URL in the future. 301 redirects tend to get cached and are hard to undo.
 	 * Client behavior for methods other than GET/HEAD is not well-defined and this type
 	 * of response should be avoided in such cases.
-	 * @param string $target Redirect URL (can be relative)
+	 * @param string $target Redirect target (an absolute URL)
 	 * @return Response
 	 */
 	public function createPermanentRedirect( $target ) {
@@ -87,12 +87,28 @@ class ResponseFactory {
 		return $response;
 	}
 
+	/**
+	 * Creates a temporary (302) redirect.
+	 * HTTP 302 was underspecified and has been superseded by 303 (when the redirected request
+	 * should be a GET, regardless of what the current request is) and 307 (when the method should
+	 * not be changed), but might still be needed for HTTP 1.0 clients or to match legacy behavior.
+	 * @param string $target Redirect target (an absolute URL)
+	 * @return Response
+	 * @see self::createTemporaryRedirect()
+	 * @see self::createSeeOther()
+	 */
+	public function createLegacyTemporaryRedirect( $target ) {
+		$response = $this->createRedirectBase( $target );
+		$response->setStatus( 302 );
+		return $response;
+	}
+
 	/**
 	 * Creates a temporary (307) redirect.
 	 * This indicates that the operation the client was trying to perform can temporarily
 	 * be achieved by using a different URL. Clients will preserve the request method when
 	 * retrying the request with the new URL.
-	 * @param string $target Redirect URL (can be relative)
+	 * @param string $target Redirect target (an absolute URL)
 	 * @return Response
 	 */
 	public function createTemporaryRedirect( $target ) {
@@ -106,7 +122,7 @@ class ResponseFactory {
 	 * This indicates that the target resource might be of interest to the client, without
 	 * necessarily implying that it is the same resource. The client will always use GET
 	 * (or HEAD) when following the redirection. Useful for GET-after-POST.
-	 * @param string $target Redirect URL (can be relative)
+	 * @param string $target Redirect target (an absolute URL)
 	 * @return Response
 	 */
 	public function createSeeOther( $target ) {
diff --git a/tests/phpunit/includes/Rest/ResponseFactoryTest.php b/tests/phpunit/includes/Rest/ResponseFactoryTest.php
index 6ccacda717..ae71272f6b 100644
--- a/tests/phpunit/includes/Rest/ResponseFactoryTest.php
+++ b/tests/phpunit/includes/Rest/ResponseFactoryTest.php
@@ -49,6 +49,13 @@ class ResponseFactoryTest extends MediaWikiTestCase {
 		$this->assertSame( 301, $response->getStatusCode() );
 	}
 
+	public function testCreateLegacyTemporaryRedirect() {
+		$rf = new ResponseFactory;
+		$response = $rf->createLegacyTemporaryRedirect( 'http://www.example.com/' );
+		$this->assertSame( [ 'http://www.example.com/' ], $response->getHeader( 'Location' ) );
+		$this->assertSame( 302, $response->getStatusCode() );
+	}
+
 	public function testCreateTemporaryRedirect() {
 		$rf = new ResponseFactory;
 		$response = $rf->createTemporaryRedirect( 'http://www.example.com/' );